London | ITP-May-2025 | Seddiq Azam | Module-Structuring-and-Testing-Data | Sprint-3 #621
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
modified: Sprint-3/2-mandatory-rewrite/1-get-angle-type.js
modified: Sprint-3/2-mandatory-rewrite/1-get-angle-type.test.js
modified: Sprint-3/2-mandatory-rewrite/1-get-angle-type.test.js
modified: Sprint-3/2-mandatory-rewrite/2-is-proper-fraction.test.js
modified: Sprint-3/2-mandatory-rewrite/3-get-card-value.test.js
modified: Sprint-3/3-mandatory-practice/implement/count.test.js
modified: Sprint-3/3-mandatory-practice/implement/get-ordinal-number.test.js
modified: Sprint-3/3-mandatory-practice/implement/repeat.test.js
sedazam
added
📅 Sprint 3
new file: package-lock.json modified: package.json
cjyuan
reviewed
Jul 6, 2025
Comment on lines
3
to
6
| // if (numerator > denominator) return false; | ||
| // if (numerator === denominator) return false; | ||
| if (denominator === 0) throw new Error("Denominator cannot be zero"); | ||
| if (numerator * denominator <= 0) return false; // Check for negative fractions or zero |
There was a problem hiding this comment.
In mathematics, -A/B == A/-B == -(A/B), and -A/-B == A/B for any integers A and B (B ≠ 0).
They represent a proper fraction if A < B and A ≠ 0 and B ≠ 0.
-2/3 is considered a proper fraction.
There was a problem hiding this comment.
You haven't made any change to your code.
isProperFraction(-2, 3) should return true.
Sprint-3/3-mandatory-practice/implement/get-ordinal-number.test.js
Outdated
Show resolved
Hide resolved
Comment on lines
5
to
24
| let result; | ||
| if (num === 1) { | ||
| return "1st"; | ||
| } else if (num === 2) { | ||
| return "2nd"; | ||
| } else if (num === 3) { | ||
| return "3rd"; | ||
| } else if (num >= 4 && num <= 20) { | ||
| return num + "th"; | ||
| } else if (num % 10 === 1) { | ||
| // % gives the last digit of the number | ||
| return num + "st"; | ||
| } else if (num % 10 === 2) { | ||
| return num + "nd"; | ||
| } else if (num % 10 === 3) { | ||
| return num + "rd"; | ||
| } | ||
| const s = ["th", "st", "nd", "rd"]; | ||
| const v = num % 100; // Get the last two digits of the number | ||
| result = num + (s[(v - 20) % 10] || s[v] || s[0]); // Determine the correct suffix based on the last two digits |
There was a problem hiding this comment.
-
The ordinal of 111 is "111th".
-
Can you explain how the code at line 24 works?
There was a problem hiding this comment.
Can you also address this comment and question?
-
The ordinal of 111 is "111th" (Can you check what your function returns when
numis 111?) -
Can you explain how the code at line 24 works?
cjyuan
added
Reviewed
…angle Sprint-3/2-mandatory-rewrite/1-get-angle-type.js
…te/3-get-card-value.js modified: Grouped multi tests Sprint-3/2-mandatory-rewrite/3-get-card-value.test.js modified: Answered questions Sprint-3/3-mandatory-practice/implement/get-ordinal-number.test.js
cjyuan
reviewed
Jul 7, 2025
There was a problem hiding this comment.
Could you also practice responding to comments directly in the comment threads? It could make tracking the discussion easier.
Here is a simplified version of best practices ChatGPT suggested for responding to inline comments in a pull request:
- ✅ Reply to every comment – Let the reviewer know you saw it.
- ✏️ Make the change if needed – Fix the code if the comment points out a real issue.
- 🤔 Explain if you don't agree – If you think the code is fine, politely explain why.
- ✅ Mark as resolved when done – Only mark comments resolved after you fix or respond.
- 💬 Keep replies short and polite – Be respectful and to the point.
- ⏱️ Respond soon – Don’t wait too long to reply.
- 🧪 Test your changes – Make sure your fixes actually work.
- 📍 Reply directly under the comment – This keeps the conversation easy to follow.
Sprint-3/3-mandatory-practice/implement/get-ordinal-number.test.js
Outdated
Show resolved
Hide resolved
Comment on lines
5
to
24
| let result; | ||
| if (num === 1) { | ||
| return "1st"; | ||
| } else if (num === 2) { | ||
| return "2nd"; | ||
| } else if (num === 3) { | ||
| return "3rd"; | ||
| } else if (num >= 4 && num <= 20) { | ||
| return num + "th"; | ||
| } else if (num % 10 === 1) { | ||
| // % gives the last digit of the number | ||
| return num + "st"; | ||
| } else if (num % 10 === 2) { | ||
| return num + "nd"; | ||
| } else if (num % 10 === 3) { | ||
| return num + "rd"; | ||
| } | ||
| const s = ["th", "st", "nd", "rd"]; | ||
| const v = num % 100; // Get the last two digits of the number | ||
| result = num + (s[(v - 20) % 10] || s[v] || s[0]); // Determine the correct suffix based on the last two digits |
There was a problem hiding this comment.
Can you also address this comment and question?
-
The ordinal of 111 is "111th" (Can you check what your function returns when
numis 111?) -
Can you explain how the code at line 24 works?
…/implement/get-ordinal-number.test.js
…2-mandatory-rewrite/2-is-proper-fraction.js modified: Sprint-3/2-mandatory-rewrite/2-is-proper-fraction.test.js
modified: Sprint-3/2-mandatory-rewrite/3-get-card-value.test.js
|
Changes look good. Well done! |
cjyuan
added
Complete
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Learners, PR Template
Self checklist
Changelist
Briefly explain your PR.
This PR includes updates and changes made across multiple Sprint 3 exercises, which include:
1-get-angle-type.jsImproved logic clarity and added corresponding test updates.
2-is-proper-fraction.jsRefined logic to correctly handle negative values and exceptional cases (e.g.,
-3/-4as a proper fraction).3-get-card-value.jsEnhanced handling for edge cases and improved code readability.
Practice Implementations
Added and updated various files (e.g.,
count.js,get-ordinal-number.js,repeat.js) to meet assignment requirements and ensure consistent behaviour.All updated files were tested to ensure correctness and alignment with the task specifications.
Questions
Ask any questions you have for your reviewer.